fix(discussions): emit REQUEST_DISCUSSION_HIGHLIGHTS when iframe CSLPs mutate#583
fix(discussions): emit REQUEST_DISCUSSION_HIGHLIGHTS when iframe CSLPs mutate#583karancs06 wants to merge 2 commits intodevelop_v4from
Conversation
🔒 Security Scan Results
⏱️ SLA Breach Summary
✅ BUILD PASSED - All security checks passed |
Coverage Report
File Coverage
|
||||||||||||||||||||||||||||||||||||||||||||||||||
…EQUEST_DISCUSSION_HIGHLIGHTS) When the user switches variants, the iframe's CSR app re-renders asynchronously and mutates every data-cslp attribute. If the visual editor pushes comment-icon highlights before those mutations settle, icons anchor to stale CSLP elements and drift permanently, or the SDK's internal retry window (900 ms) expires before the correct element appears and no icon is mounted. This change inverts the coordination: the SDK's existing variant-class MutationObserver now ALSO emits REQUEST_DISCUSSION_HIGHLIGHTS (debounced 200 ms) whenever data-cslp attributes mutate. The visual editor treats that as a pull request and re-sends the highlight payload against the now-final DOM. Because the payload already contains both base and variant CSLP rows, whichever one matches the current DOM wins. Changes in this repo: - postMessage.types.ts: add REQUEST_DISCUSSION_HIGHLIGHTS event. - useRecalculateVariantDataCSLPValues.ts: debounce-emit the event from the existing per-element MutationObserver callback. - useVariantsPostMessageEvent.ts: for SSR apps the DOM is already final so emit immediately; for CSR apps the observer handles it. - generateHighlightedComment.tsx: updateHighlightedCommentIconPosition now removes orphaned icons whose target [data-cslp] has disappeared, instead of silently leaving them drifted. Tests: 3 new SSR-path tests plus a 5-test file for the orphan cleanup in generateHighlightedComment. The MutationObserver → debounce → postMessage path is an integration guarantee — jsdom does not fire attribute mutations reliably, so it is verified in the visual-editor DiscussionPage tests where the listener is driven directly. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
e189f93 to
f03cee9
Compare
🔒 Security Scan Results
⏱️ SLA Breach Summary
✅ BUILD PASSED - All security checks passed |
| DATA_CSLP_ATTR_SELECTOR | ||
| ); | ||
| updateElementClasses(element, dataCslp || "", observer); | ||
| requestDiscussionHighlights(); |
There was a problem hiding this comment.
we can debounce it right? I don't want us to spam, postmessages from LP SDK through mutation observer
There was a problem hiding this comment.
Yes, it's already debounced — requestDiscussionHighlights schedules a single 200ms timer (DISCUSSION_HIGHLIGHTS_DEBOUNCE_MS) and resets it on every observed mutation, so a burst of data-cslp mutations from a CSR re-render coalesces into exactly one postMessage. See lines 14–17 (constant) and 47–57 (debounce wrapper) in this file.
If you'd prefer a longer window (say 300ms or 500ms) for extra safety against very chunky React renders, easy to bump.
| const requestDiscussionHighlights = () => { | ||
| if (highlightsRequestTimer !== null) clearTimeout(highlightsRequestTimer); | ||
| highlightsRequestTimer = setTimeout(() => { | ||
| highlightsRequestTimer = null; | ||
| visualBuilderPostMessage?.send( | ||
| VisualBuilderPostMessageEvents.REQUEST_DISCUSSION_HIGHLIGHTS | ||
| ); | ||
| }, DISCUSSION_HIGHLIGHTS_DEBOUNCE_MS); | ||
| }; |
There was a problem hiding this comment.
I would have used debounce from lodash instead of custom logic.
🔒 Security Scan Results
⏱️ SLA Breach Summary
✅ BUILD PASSED - All security checks passed |
Problem
When the user switches variants, the iframe's CSR app re-renders asynchronously and mutates every
data-cslpattribute. If the visual editor pushes comment-icon highlights optimistically (before those mutations settle), two failure modes follow:Solution — pull-based, SDK-driven
This PR mirrors the highlight-variant pattern: the MutationObserver lives in the iframe (only place it can live because the iframe may be cross-origin from the visual editor), detects
data-cslpchanges, and signals the visual editor to re-send the highlight list. The VB owns the field-path list, so the SDK cannot re-mount icons on its own — it can only ask for a refresh.1.
REQUEST_DISCUSSION_HIGHLIGHTSpostMessage event (postMessage.types.ts)Semantically names what the iframe is saying: "my CSLPs moved, please re-sync highlights."
2. Debounced emit from the existing observer (
useRecalculateVariantDataCSLPValues.ts)updateVariantClassesalready sets up aMutationObserverper[data-cslp]element. We add a single fire-and-forget emit, debounced 200 ms so a burst of mutations coalesces into one request:3.
useVariantFieldsPostMessageEvent— SSR path (useVariantsPostMessageEvent.ts)For SSR pages the DOM is already in its final state when the variant arrives —
updateVariantClasses' observer will never fire. Emit the request immediately:4. Orphan cleanup in
updateHighlightedCommentIconPosition(generateHighlightedComment.tsx)Previously, if an icon's
field-pathno longer matched any[data-cslp]in the DOM (e.g. after a variant switch mutated attributes), the function silently skipped it — the icon stayed visible at its old position and drifted during scroll. Now it's removed immediately:This is independently correct even without the new event.
Why a pull (not a push)?
An earlier approach had the SDK emit a "DOM settled, proceed" gate event that the visual editor used to unblock a prequeued push. That worked but forced VB to carry cslpSettled state, a 5-second fallback, and an
isVariantChangeRefskip-on-mount guard. The pull pattern is stateless on the VB side (just a listener) and self-heals for any cause of CSLP mutation — hydration, dynamic content, route changes — not only variant switches.Tests
useVariantsPostMessageEvent.spec.tsREQUEST_DISCUSSION_HIGHLIGHTSfor SSR+variant, SSR+null variant; does NOT emit directly for CSR (observer handles it)generateHighlightedComment.test.ts(new file, 5 tests)removeAllHighlightedCommentIconsThe
MutationObserver → debounce → postMessagechain is an integration guarantee — jsdom does not reliably fire attribute mutations, so that end-to-end path is verified in the visual-editorDiscussionPage.spec.tsxtests (companion PR) where the listener is driven directly.Full suite: 756 tests pass.
Companion PR
Visual editor: contentstack/visual-builder#2102
🤖 Generated with Claude Code